-
Notifications
You must be signed in to change notification settings - Fork 739
feat(notifications): notifications controller and view panel #5828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions. |
bc8edd2 to
736901c
Compare
- NotificationsController - Fetches notifications and determines which ones to display. - NotificationsNode - Side panel that displays a given list (from controller) of notifications. The controller will fetch notifications from an endpoint (or local files during development). These notifications will be stored in memory and global state. They will then be evaluated for dismissal/criteria and if they pass they will be send to the NotificationsNode for display.
736901c to
15c636c
Compare
| @@ -0,0 +1,9 @@ | |||
| /*! | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
somewhat off-topic, but would it avoid boilerplate if the main/top-level core index.ts file instead just exports all notifications stuff under a notifications item? that has at least these advantages:
- avoids needing to update
package.json - avoids needing to create this file
- centralizes the index
- avoids consumers having to hunt for index files (they import one "mega index")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to a top-level index.js, you would still need a lower level one to collect the individual files though right?
e.g.
src/notifications/index.ts
export * from 'controller.ts'
export * from 'types.ts'
src/index.ts
export * as notifications from 'notifications/index'
Maybe you could do this, but this looks like it would be very messy for many exports:
src/index.ts
import * as controller from './src/notifications/controller';
import * as types from './src/notifications/types';
export const notifications = {
...controller,
...types,
}
Also, not sure how autocomplete will behave. Needs investigation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you would still need a lower level one to collect the individual files though right?
e.g.
no, could do that manually in the top level index.ts like this:
aws-toolkit-vscode/packages/core/src/shared/index.ts
Lines 50 to 51 in 6fa3a9e
| export * as messages from './utilities/messages' | |
| export * as errors from './errors' |
of course, this means the top level has to "choose" some names, but in practice that doesn't seem to be a problem, and it may actually lead to a better interface for the consumers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So then you have pretty granular exports anyways, right? e.g.
export * as notificationsTypes from './src/notifications/types'
export * as notificationsRules from './src/notifications/rules'
export * as notificationsController from './src/notifications/controller'
which isn't as intuitive as a single notifications export from all of these places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, though it depends on what consumers actually need. Generally we shouldn't need to expose most modules in the "public" interface.
Tracked in https://taskei.amazon.dev/tasks/IDE-15174
The controller will fetch notifications from an endpoint (or local files during development). These notifications will be stored in memory and global state. They will then be evaluated for dismissal/criteria and if they pass they will be sent to the NotificationsNode for display.
Extensions will call
NotificationController.pollForStartUp()andNotificationController.pollForEmergencies()to get notifications.License: I confirm that my contribution is made under the terms of the Apache 2.0 license.